Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP add a couple of types for working with patches #357

Closed
wants to merge 3 commits into from

Conversation

danbornside
Copy link
Contributor

Patchable generalizes PatchMap/SemiMap to any patchable type.
PatchDMapWithReset allows to use patches in keys. I didn't also solve the WithMove part since that adds significant typelevel overhead I didn't happen to need.

mergeWithKey f g fg = \xs ys -> DMap.mapMaybeWithKey onlyThat $ DMap.unionWithKey doIt (DMap.map This1 xs) (DMap.map That1 ys)
where
doIt _ (This1 xs) (That1 ys) = These1 xs ys
doIt _ _ _ = error "mergeWithKey misalligned keys"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

misaligned

-- | A set of changes to a 'DMap'. Any element may be inserted/updated or deleted.
-- Insertions are represented as @'ComposeMaybe' (Just value)@,
-- while deletions are represented as @'ComposeMaybe' Nothing@.
newtype PatchDMapWithReset k p = PatchDMapWithReset { unPatchDMapWithReset :: DMap k (By p) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the connection with ComposeMaybe? Leftover from vanilla PatchDMap?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name "WithReset" - what is "Reset" in this case?

I think there's a need for this type (also for normal Map), I've seen a few instances of this type in different places...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also didn't know what Reset meant. Even if it has some precedence somewhere out there, the fact that none of us understand it means we should probably change it.

inserts k = has @(Patches1Locally p) k $ \case
By_Insert x -> Just x
By_Delete -> Nothing
By_Patch _ -> Nothing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What to do when the patch is invalid? Is it better to call error (which seems kind of ugly... )?

The use of Maybe as documented seems to be more about Patches which don't cause any updates, rather than for error handling - I guess this is the only sensible thing to do though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when is a patch invalid?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example when applying a patch "By_Patch" to a key which doesn't exist...

@Ericson2314
Copy link
Member

Superceded by reflex-frp/patch#2

@Ericson2314 Ericson2314 closed this Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants